Skip to content

Conversation

@facutuesca
Copy link
Contributor

This PR adds a new WriteError enum variant CustomError, for users that implement Asn1Writable::write for their own types and have errors that can happen during encoding that are not covered by the current enum variants.

@facutuesca facutuesca force-pushed the ft/new-write-error branch from 8c6daef to 5ebf95d Compare June 22, 2025 10:15
@alex
Copy link
Owner

alex commented Jun 22, 2025 via email

@alex
Copy link
Owner

alex commented Jun 23, 2025

Hmm, stepping back a bit, this design isn't super consistent with what we have for parsing. Can you show a bit of code that explains why/how this is required, and then we can brainstorm on if there's a better way?

@facutuesca
Copy link
Contributor Author

facutuesca commented Jun 23, 2025

Hmm, stepping back a bit, this design isn't super consistent with what we have for parsing. Can you show a bit of code that explains why/how this is required, and then we can brainstorm on if there's a better way?

Sure! This is in the context of the Python ASN.1 API that uses rust-asn1. In order to encode a Python object, we first convert it to a Rust object:

pub struct AnnotatedTypeObject<'a> {
    // This contains the Python type information, that determines what type of ASN.1 structure
    // this represents (e.g.: a SEQUENCE with two fields, one of them has a DEFAULT value, etc)
    pub(crate) annotated_type: &'a AnnotatedType,
    // The actual value to encode (e.g.: an object with two attributes, one per field)
    pub(crate) value: &'a Bound<'a, PyAny>,
}

Then we implement asn1::Writable for this type:

impl asn1::Asn1Writable for AnnotatedTypeObject<'_> {
    fn write(&self, writer: &mut Writer<'_>) -> Result<(), asn1::WriteError> {
        Python::with_gil(|py| {
            let annotated_type = self.annotated_type;
            let inner = annotated_type.inner.get();

            match &inner {
                Type::Sequence(cls) => {
                    let fields = cls
                        .getattr(py, "__asn1_fields__")
                        .map_err(|_| asn1::CustomError("Error accessing SEQUENCE fields")?;
                     //...etc, encode the values using `asn1::SequenceWriter::new`
                    // this also recursively calls `write` for each field
                 }
                 Type::PyInt() => {
                    let val: i64 = value
                        .extract()
                        .map_err(|_| asn1::CustomError("Error converting Python int to Rust i64")?;
                    val.write(writer)
                 }
                 //etc (other types)

Since we traverse the ASN1 structure on the fly while we write/encode the values, there are errors that can happen inside the write() trait method that are not encoding errors per-se, but rather conversion errors between the Python and Rust type representations.

@alex
Copy link
Owner

alex commented Jun 29, 2025

Ok, I spent some time thinking, and I wonder if adding an assosciated Error type to Asn1Writable, which defaults to WriteErorr and has a From<WriteError> bound would work?

@facutuesca
Copy link
Contributor Author

Ok, I spent some time thinking, and I wonder if adding an assosciated Error type to Asn1Writable, which defaults to WriteErorr and has a From<WriteError> bound would work?

looks like default values for associated types are unstable:

pub trait Asn1Writable: Sized {
    type Error = WriteError;
            └─associated type defaults are unstable
              see issue #29661 <https://github.com/rust-lang/rust/issues/29661> for more information

@alex
Copy link
Owner

alex commented Jun 29, 2025 via email

@facutuesca
Copy link
Contributor Author

How would you like to handle derive(asn1::Asn1Write) in that case? If each Asn1Writable type has its own associated Error, doing:

#[derive(asn1::Asn1Write)]
struct S {
    field1: MyTypeA,
    field2: MyTypeB,
}

means that the derived write function would call write functions that return Result<(), MyTypeA::Error> and Result<(), MyTypeB::Error>. So how do we define S::Error?

@alex
Copy link
Owner

alex commented Jun 30, 2025 via email

@facutuesca
Copy link
Contributor Author

Hmm, maybe an additional annotation to specify the write error type?

I started implementing this, see #567

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants